w, needClose, err := getOutputWriter()
	if err != nil {
		return err
	}
	if needClose {

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/generate.go`:
- Around line 75-84: The CLI currently allows using --base-config with non-XML
output and silently ignores it; update the validation in the generate command
(after computing format via normalizeStringFlag(outputFormat) and before
returning success) to reject any invocation that supplies baseConfig when format
is not formatXML (i.e., when format == formatCSV or default), returning a clear
error such as "--base-config is only valid with xml format"; check the flag
variable (baseConfig or baseConfigPath) and return fmt.Errorf when it's set and
format != formatXML so the CLI fails fast and informs the user.

In `@CONTRIBUTING.md`:
- Around line 120-123: The documentation currently claims a dispatcher exists
but the CLI still hardwires the OPNsense serializer in cmd/generate.go; update
the text to describe the intended architecture as planned behavior: state that
model.CommonDevice is intended as the single IR, that internal/faker/ will
populate it and internal/serializer/opnsense/ implements one serializer, and
that the package layout reserves a pfSense sibling so a future
internal/serializer/pfsense/ can plug in and the CLI will route by
device.DeviceType once implemented; do not assert that the dispatcher or device
routing is already present.

In `@internal/csvio/csvio.go`:
- Around line 46-49: The current map byPhysical :=
make(map[string]model.Interface...) uses iface.PhysicalIf as key which causes
collisions when PhysicalIf == ""; change byPhysical to
map[string][]model.Interface and populate with byPhysical[iface.PhysicalIf] =
append(byPhysical[iface.PhysicalIf], iface) so multiple interfaces (including
empty keys) are preserved; then update any downstream code that reads byPhysical
to handle a slice of model.Interface instead of a single model.Interface.

In `@internal/faker/device_test.go`:
- Around line 34-41: Add edge-case tests for VLAN count boundaries by creating
new tests that call NewCommonDevice with WithVLANCount(0) and WithVLANCount near
the upper limit (e.g. 4092); for the zero case assert dev.VLANs is empty,
dev.Interfaces length equals 2 (WAN + LAN) and dev.DHCP length equals 1, and for
the high-count case assert dev.VLANs length matches the requested count,
dev.Interfaces grows by that count (2 + count), and VLAN tag uniqueness (no
duplicates) across dev.VLANs; reference TestNewCommonDeviceVLANCount,
NewCommonDevice, WithVLANCount, dev.VLANs, dev.Interfaces, and dev.DHCP when
adding these tests.

In `@internal/faker/network.go`:
- Around line 35-39: The fakeNetwork function currently assumes vlanCount is
valid and downstream helpers pickUniqueTag/pickUniqueNet may panic on exhausted
or out-of-range VLAN space; validate vlanCount in fakeNetwork (and/or at the
start of NewCommonDevice) to ensure 0 <= vlanCount <= 4092 and return a
descriptive error instead of proceeding, change the fakeNetwork signature to
return (networkResult, error) and propagate errors from
pickUniqueTag/pickUniqueNet back to the caller (adjust NewCommonDevice to handle
the error), and update any callers of fakeNetwork to handle the error path so
invalid counts are rejected with clear messages rather than panicking.

In `@internal/faker/options.go`:
- Around line 1-38: NewCommonDevice must validate cfg.vlanCount before calling
fakeNetwork: reject values <0 with error "VLAN count must be >= 0" and >4092
with error "VLAN count must be <= 4092". Change NewCommonDevice to return
(*model.CommonDevice, error) (update its callers) and perform the checks on
cfg.vlanCount early; if invalid, return nil and the corresponding error,
otherwise proceed to call fakeNetwork and return the created *model.CommonDevice
and nil error. Ensure the validation references cfg.vlanCount and the
NewCommonDevice function name so future edits are clear.

In `@internal/faker/rand_test.go`:
- Around line 25-31: The test TestNewRandZeroSeedIsRandom is flaky because it
compares a single Uint64() draw from two streams; change it to sample multiple
times (e.g., loop N times) from the two rand.Rand instances returned by
newRand(0) and assert that at least one pair of corresponding draws differs (or
assert divergence within a small number of draws), referencing newRand and
TestNewRandZeroSeedIsRandom so the test is hardened against probabilistic
collisions while preserving the intent that zero-seed streams diverge.

In `@internal/faker/system_test.go`:
- Around line 49-59: TestFakeSystemDomainIsLowercaseFQDN currently only uses
seed 17; update it to iterate the same seed set used in
TestFakeSystemHostnameIsDNSSafe (e.g., {1,2,3,42,100}) and run the newRand(seed)
-> fakeSystem(f) flow for each seed so sys.Domain is validated across multiple
seeds; keep the same assertions (no uppercase runes and contains a dot) and
reference the existing helpers newRand and fakeSystem while iterating seeds
inside TestFakeSystemDomainIsLowercaseFQDN.

In `@internal/opnsensegen/commondevice_test.go`:
- Around line 49-56: Add a few extra assertions in commondevice_test.go after
the existing checks to spot-check additional fields for stronger round-trip
parity: assert that device.System.Timezone and device.System.Language equal
original.System.Timezone and original.System.Language (use assert.Equal with the
same pattern as Hostname/Domain), and inspect VLANs on device.VLANs to assert
the expected tags/IDs for at least one or two entries (e.g., check
device.VLANs[0].Tag and .ID or a VLAN with a known name), and optionally assert
a specific interface property from device.Interfaces (like a first interface
Name or MAC) to ensure deeper coverage while keeping the zero-warnings gate.

In `@internal/serializer/opnsense/dhcp_test.go`:
- Around line 35-40: The test TestSerializeDHCPDisabled is currently asserting
out.Items["lan"].Enable which will succeed if the "lan" key is missing due to
Go's zero-value behavior; change the test to first verify the key exists in the
map returned by SerializeDHCP (e.g., check that "lan" is present using a map
lookup or assert.Contains) and only then assert that out.Items["lan"].Enable
indicates disabled; reference SerializeDHCP, TestSerializeDHCPDisabled and the
out.Items["lan"].Enable lookup when making the change.

In `@internal/serializer/opnsense/interfaces_test.go`:
- Around line 51-56: The test TestSerializeInterfacesDisabled uses
serializer.SerializeInterfaces and then directly asserts fields on
out.Items["lan"], which can falsely pass if the "lan" key is missing; update the
test to first assert that the map contains the expected key (e.g., check
presence of "lan" in out.Items using a key-existence assertion) and only then
assert the Enable field value; apply the same change to the other negative-path
test(s) that reference "opt1" so they verify key presence before inspecting
fields.

In `@internal/serializer/opnsense/overlay.go`:
- Around line 23-25: The Serialize(device) call in overlay.go returns err which
is propagated directly; change the error return to wrap it with context (e.g.,
use fmt.Errorf) so failures show overlay serialization context — update the
return path after serialized, err := Serialize(device) to return nil,
fmt.Errorf("overlay serialize: %w", err) (and add the fmt import if missing).

In `@internal/serializer/opnsense/vlans_test.go`:
- Around line 11-26: Test covers first VLAN only; add assertions for the second
serialized VLAN to validate iteration: after calling
serializer.SerializeVLANs(in) in TestSerializeVLANs, add at least a spot-check
like assert.Equal(t, "100", out.VLAN[1].Tag) (optionally also assert.If/Descr
using out.VLAN[1].If and out.VLAN[1].Descr) to confirm the second entry was
serialized correctly.

In `@README.md`:
- Line 52: Update the README sentence to clarify that routing by
CommonDevice.DeviceType is planned rather than implemented: change the
description around the generate CLI path to state that currently the CLI still
hardwires the opnsense serializer (internal/serializer/opnsense) but will in
future dispatch to a serializer based on CommonDevice.DeviceType, allowing a
sibling internal/serializer/pfsense to be selected; reference the generate
command/CLI routing and the CommonDevice.DeviceType selector so readers know
this is intended behavior, not current functionality.

---

Outside diff comments:
In `@cmd/generate.go`:
- Around line 94-104: The current code opens the output destination via
getOutputWriter() before performing base-config loading, overlaying, and XML/CSV
rendering, which can create/truncate the target even if rendering fails; change
the flow so rendering happens into memory first (e.g., produce a bytes.Buffer or
[]byte from the base-config loading, overlaying, and XML/CSV rendering steps),
and only after rendering succeeds call getOutputWriter(), write the rendered
bytes, and then close the writer (handling close errors as before). Apply the
same change to the other opening/writing block mentioned (the similar code at
the 118-129 region) so no output file is created/truncated before successful
rendering.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/faker/dhcp_test.go`:
- Around line 21-38: The test currently only checks DHCP range endpoints; update
the happy-path in dhcp_test.go to also assert each scope's Gateway and DNSServer
are set and correct: for each scope returned by fakeDHCPScopes, assert s.Gateway
and s.DNSServer are non-empty, parseable via netip.ParseAddr, and match the
interface IP that was used to create that scope (use the original interfaces
input to look up the expected IP for the scope.Interface name). Keep these
checks inside the existing loop that validates s.Range so they run per-scope.

In `@internal/opnsensegen/commondevice_test.go`:
- Around line 31-57: The test never exercises filter/firewall rules because
faker.NewCommonDevice was called without enabling rules; update the
faker.NewCommonDevice call to include faker.WithFirewallRules(true) alongside
faker.WithSeed(146) and faker.WithVLANCount(2), then after ConvertDocument
succeeds add an assertion that at least one rule survived (e.g.
require.NotEmpty(t, device.Firewall.Rules) or assert.NotEmpty(t,
device.Firewall.Rules)) to guarantee the serializer → opnsensegen.MarshalConfig
→ opnsensegen.ParseConfig → opnsenseparser.ConvertDocument round-trip preserves
firewall rules.

In `@internal/serializer/opnsense/overlay_test.go`:
- Around line 16-44: The test only verifies System and VLANs but not
Dhcpd/Filter so seed both Dhcpd and Filter in the base OpnSenseDocument fixture
(the base variable of type opnschema.OpnSenseDocument) with representative
items, recreate the device via faker.NewCommonDevice using
faker.WithFirewallRules(true) (in addition to faker.WithVLANCount(2)) so the
device contains firewall/DHCP state, call serializer.Overlay(base, device) as
before, and add assertions that merged.Dhcpd (or merged.Dhcpd.* fields you
seeded) and merged.Filter (or merged.Filter.* rules) equal the corresponding
values from device to ensure Overlay replaces those Phase 1 subsystems from the
device.

In `@internal/serializer/opnsense/serializer_test.go`:
- Around line 92-105: The current per-field parity loop in TestRoundTrip (using
firewallRulesByInterface to map original.FirewallRules and
roundTripped.FirewallRules) only checks Type/Description/IPProtocol/Addresses;
add assert.Equalf checks inside the same for iface, want := range origRules loop
to compare want.Direction vs got.Direction, want.Protocol vs got.Protocol,
want.Log vs got.Log, want.Disabled vs got.Disabled, want.Tracker vs got.Tracker
and also compare want.Source.Port vs got.Source.Port, want.Source.Negated vs
got.Source.Negated, want.Destination.Port vs got.Destination.Port and
want.Destination.Negated vs got.Destination.Negated (use the same assert.Equalf
pattern and message format as the existing assertions so missing fields in
SerializeFilter are detected).

---

Duplicate comments:
In `@cmd/generate.go`:
- Around line 74-84: The CLI currently allows --base-config with non-xml output
because validation for base-config is never performed in runGenerate; add an
explicit check in runGenerate (after determining format from
normalizeStringFlag(outputFormat) and the format switch) to return an error when
the user passed the base-config flag (e.g., variable representing --base-config)
while format != formatXML; use a clear message like "`--base-config` is only
valid with --format xml`" and return fmt.Errorf(...) so the command fails fast
instead of silently ignoring the flag.

In `@internal/faker/network.go`:
- Around line 39-43: The function fakeNetwork currently allocates slices using
vlanCount before validating it, which can panic for negative values or allow
callers (including NewCommonDevice) to bypass CLI guards and cause wasted
retries; add an upfront validation in fakeNetwork that checks vlanCount is
within the allowed range (e.g., 0..4092) and return an error immediately if out
of range, before constructing networkResult and allocating Interfaces/VLANs, so
callers receive a proper error instead of a panic or unnecessary loops.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/cmd_test.go`:
- Around line 22-35: The test helper fails to reset three package-level validate
flags—inputFile, inputFormat, and maxErrors—so state leaks between tests; update
the reset block in newTestRootCmd (or wherever the other flag globals like
outputFormat, vlanCount, includeFirewall, etc. are reset) to also assign
inputFile = "", inputFormat = <defaultInputFormat> (or the correct default
constant), and maxErrors = <defaultMaxErrors> so validate-related tests are
order-independent.

In `@cmd/generate.go`:
- Around line 123-132: The current code always calls
serializer.Serialize(device) and then discards its result when baseConfigPath is
provided; instead, branch on baseConfigPath before serializing so the device is
only serialized once: if baseConfigPath != "" first call
opnsensegen.LoadBaseConfig(baseConfigPath) and then call
serializer.Overlay(base, device) to produce doc; otherwise call
serializer.Serialize(device) to produce doc. Update the logic around
baseConfigPath, serializer.Serialize, serializer.Overlay and
opnsensegen.LoadBaseConfig so doc is only produced once from the appropriate
path.

In `@internal/faker/device.go`:
- Around line 59-66: The faker currently hard-codes model.DeviceTypeOPNsense
into the intermediate CommonDevice struct in NewCommonDevice; remove that
target-specific assignment so NewCommonDevice returns a device-agnostic
*model.CommonDevice with DeviceType left unset (zero value) instead of
model.DeviceTypeOPNsense, and ensure downstream serializers are responsible for
setting DeviceType; update any callers/tests that incorrectly assume
NewCommonDevice populated DeviceType to use serializer injection or explicit
setting before serialization.

In `@README.md`:
- Around line 24-26: The fenced code block showing the shell command
"opnconfiggenerator generate --base-config existing.xml --seed 42" in README.md
is missing a language specifier; update that opening triple-backtick to include
a language (use "text" or "console") so static analysis passes (e.g., change ```
to ```text) and apply the same fix to the similar block around line 66.

---

Duplicate comments:
In `@internal/serializer/opnsense/overlay_test.go`:
- Around line 86-98: The test currently only asserts base values were removed
(using merged.Dhcpd.Items["lan"] and merged.Filter.Rule) but must assert the
merged sections equal the device payload; update the assertions to compare
merged.Dhcpd.Items["lan"] (including Gateway) to the device's Dhcpd "lan" entry
from the serialized device fixture and assert merged.Filter.Rule equals (or
deep-equals) the device's Filter.Rule slice, replacing the NotEqual checks so
Overlay wholesale-replacement is validated against the actual device document
rather than merely being different from the base.

In `@internal/serializer/opnsense/serializer_test.go`:
- Around line 97-113: The parity loop currently compares rule fields keyed by
iface but misses verifying the full Interfaces slice; update the loop that
iterates over origRules (for iface, want := range origRules) to also assert
equality of want.Interfaces and got.Interfaces (the FirewallRule.Interfaces
field) for each rule, ensuring order and contents match exactly between want and
got; use the existing test helpers (e.g., assert.Equalf or require.Equalf)
referencing the variables want.Interfaces and got.Interfaces and include a
descriptive message like "rule %q Interfaces" to make failures clear.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/opnsensegen/template.go`:
- Around line 177-205: The warning messages emitted in the depth==0 switch in
internal/opnsensegen/template.go (the cases handling xml.CharData, xml.Comment,
xml.ProcInst, xml.Directive and using start.Name.Local) are too terse; update
each log.Warn call to include a clear, actionable instruction (e.g. mention the
dropped token type, the affected section from start.Name.Local, and what the
operator should do to preserve the annotation such as "move this comment into
managed section or use override/patch mechanism"), and for xml.ProcInst include
tok.Target as now but append the same actionable guidance so operators know how
to avoid losing annotations.